Improve performance 2 - #136
Merged
Merged
Conversation
Changepacksvespera@0.2.0 → 0.3.0 - Cargo.tomlMinor
vespera@0.2.0 → 0.3.0 - crates/vespera/Cargo.tomlMinor
vespera_core@0.2.0 → 0.3.0 - crates/vespera_core/Cargo.tomlMinor
vespera_inprocess@0.2.0 → 0.3.0 - crates/vespera_inprocess/Cargo.tomlMinor
vespera_jni@0.2.0 → 0.3.0 - crates/vespera_jni/Cargo.tomlMinor
vespera_macro@0.2.0 → 0.3.0 - crates/vespera_macro/Cargo.tomlMinor
vespera-bridge@0.2.0 → 0.3.0 - libs/vespera-bridge/build.gradle.ktsMinor
vespera-bridge-gradle-plugin@0.1.1 → 0.2.0 - libs/vespera-bridge-gradle-plugin/build.gradle.ktsMinor
|
…esponse wire-header serializer (BENCHMARK ITEM), Unify the duplicated CARGO_MANIFEST_DIR epoch-cache helper in the proc-macro file cac… (7/8 applied)
Applied (7/8):
- [1] Skip per-name HeaderMap hash lookups in the response wire-header serializer (BENCHMARK ITEM) — impact: All-single response-header maps retain borrowed values while sorting, eliminating one HeaderMap lookup per distinct name. Repeated-name maps retain the existing rendering path.; evidence: Candidate-specific A/B data is documented in the implementation report. My five-run full benchmark attempt timed out af…; decision: Independent review found no contract or correctness issue. Workspace tests, clippy with warnings denied, formatting, and diff checks pass; the added test covers the new all-single path on both output sinks.
- [3] Unify the duplicated CARGO_MANIFEST_DIR epoch-cache helper in the proc-macro file cache — impact: A single documented epoch-cache helper now serves both manifest-directory lookup paths, preventing future fixes from diverging between duplicate implementations.; evidence: none; this proc-macro-only refactor is not linked into the configured runtime benchmarks.; decision: Independent diff review confirmed unchanged cache semantics and correct child-to-parent private access. Workspace tests, strict clippy, formatting, diff checks, and the configured benchmark command passed; see the review report.
- [4] Reuse file_utils::{mtime_fingerprint, combine_fingerprint} in vespera_impl/cache.rs instead of two … — impact: Sidecar and route-file cache fingerprints now share one canonical mtime conversion and mtime/size mixer, preventing future cache-key divergence while preserving existing values and failure sentinels.; evidence: none; modified helpers execute during proc-macro expansion and are not reached by the requested runtime benchmark crates; decision: Independent diff review confirmed identical arithmetic and None/zero failure behavior. Workspace tests, clippy, fmt, diagnostics, and diff checks passed; each mixing constant appears once in vespera_macro. See the review report for benchma…
- [5] Extract the duplicated post-dispatch stream-abort tail shared by both dispatch*WithHeader JNI symbo… — impact: The shared success-side stream-abort predicate now has one private inline implementation, preventing the two JNI header-dispatch symbols from drifting apart while preserving their truncation-reporting contract.; evidence: none; the inline JNI-only helper is not exercised by the configured vespera and vespera_inprocess benchmarks.; decision: Independent diff review confirmed identical predicate, single Acquire snapshot, and buffer-release ordering at both call sites. Build, workspace tests, strict clippy, formatting, diff check, and five configured benchmark runs all completed…
- [6] Collapse the three repeated header-buffer sizing / serialize / 500-on-overflow blocks in wire.rs in… — impact: A private inline helper now owns all three header-vector sizing and serialization paths, preventing their capacity and overflow handling from drifting while preserving emitted wire bytes and allocations.; evidence: No attributable regression. Five affected benchmark samples had a 1.7455 us median and a noisy 1.6631-1.8173 us range; …; decision: Independent backup comparison confirmed unchanged capacity arithmetic, serializer arguments, body reserve, and 500 fallback. Workspace tests, Clippy with warnings denied, formatting, and diff checks passed; the affected benchmark showed no…
- [7] Lock the ingress-cap predicate config::exceeds with unit tests (currently zero coverage) — impact: Adds 10 inline rstest cases that lock the unlimited sentinel and strict ingress-cap boundary, preventing default-cap outages and exact-cap 413 regressions.; evidence: none; only cfg(test) code and a dev dependency changed, neither compiled into benchmark artifacts; decision: Independent review confirmed the requested matrix, no production API changes, and passing workspace tests, clippy, and formatting. Five benchmark runs exited successfully; host-noisy timings cannot affect this test-only change.
- [8] Give the hand-rendered dispatch() 500 envelope a serde drift tripwire — impact: The private 500 fallback JSON is now a documented constant guarded against drift from ResponseEnvelope and ResponseMetadata serde output.; evidence: none; same compile-time-expanded literal on an unreachable fallback path; decision: Independent mutation made the new test fail with the expected byte diff; restoration passed. Workspace tests, clippy, fmt, diff checks, and five benchmark invocations completed.
Not applied (1/8):
- [2] Remove the write-only FileCache::path_lookup_epoch field and the no-op ensure_path_lookup_caches_fr… — failed; attempted impact: Candidate removes private dead cache state and documents the active epoch and fingerprint revalidation mechanism.; evidence: unverified: runtime benchmark is not attributable to this compile-time macro change and the mandated five-run sample wa…; reason: Independent diff review, workspace tests, clippy, formatting, and removed-symbol search passed, but the required five benchmark samples did not complete before the review harness timeout. See the report.
… the no-op `ensure_path_lookup_caches_fresh`, Unify the duplicated per-crate macro storage in `route_impl` and `cron_impl`, Remove the… (6/7 applied) Applied (6/7): - [1] Remove the dead `path_lookup_epoch` field and the no-op `ensure_path_lookup_caches_fresh` — impact: Removes a write-only private cache field, its no-op wrapper, and two dead calls. Corrected documentation now describes the actual epoch-and-fingerprint freshness mechanism.; evidence: none; the private proc-macro code is not linked into the configured runtime benchmark binaries; decision: Independent diff and cache-flow review found no read of the removed field and confirmed lookup invalidation is still driven by last_epoch_validated and path fingerprints. Workspace tests, both clippy configurations, and formatting passed; … - [3] Unify the duplicated per-crate macro storage in `route_impl` and `cron_impl` — impact: Route and cron metadata registration now share one poison-tolerant, copy-on-write per-crate storage implementation, eliminating duplicated replacement logic while preserving all entry points and behavior.; evidence: none; runtime code unchanged. The attempted five-run benchmark exceeded the 30-minute command limit during run two, so …; decision: Independent code review found the generic implementation behavior-equivalent. `cargo test --workspace`, strict clippy, formatting, and diff checks pass. The full five-pass runtime benchmark exceeded the command time limit; the change is co… - [4] Remove the two `unwrap()` panic sites in `process_vespera_macro` — impact: Cache freshness is now represented by `Option<VesperaCache>`, so stale caches cannot reach reuse and the two non-test unwrap panic sites are removed.; evidence: No candidate-caused runtime change: this is a proc-macro control-flow refactor; the cached OpenAPI output is byte-ident…; decision: Independent workspace tests, clippy with warnings denied, and formatting passed. Two forced profiled axum-example recompilations took the cache-hit stages and preserved the OpenAPI SHA-256; five configured benchmarks completed, with unrela… - [5] Stop cloning route path/name into the error-only `claimed` map in `assemble_path_items` — impact: The duplicate-route map now borrows route paths and handler names from immutable metadata, eliminating two successful-path String allocations per discovered route.; evidence: none; this private compile-time proc-macro refactor is not linked into the configured runtime benchmark binaries; decision: Independent workspace tests, clippy, format, and the focused duplicate-route diagnostic test passed. The map borrows metadata that outlives the assembly loop, and the unchanged format string preserves the diagnostic text. See the review re… - [6] Return a borrow from `MergeSpecCache::read` instead of cloning the child spec — impact: The cache now returns its stored entry by reference, removing full serialized child-spec clones on the cache miss and on every later read. This reduces compile-time allocation and copying while preserving merge and config-hash behavior.; evidence: Compile-time: removes two full child-spec allocations and memcpys across the hash miss and later merge read per child a…; decision: Independent review found both consumers read-only and the ownership change sound. `cargo test --workspace`, clippy with warnings denied, fmt check, and an axum-example merge build passed; regenerated openapi.json was unchanged. The runtime… - [7] Delete the unfulfilled `#[allow]` on the test-only `collect_metadata` wrapper — impact: Removed a dead lint suppression from the test-only collector wrapper, restoring accurate lint coverage without changing behavior.; evidence: none; five benchmark passes completed but host contention produced unusable variance, while this cfg(test) lint attribu…; decision: Independent review confirmed the one-line requested diff, strict all-target Clippy and formatting passed, and the full workspace test suite passed. See the review report for benchmark evidence and limitations. Not applied (1/7): - [2] Drop the always-None header_bytes_owner parameter from internal::dispatch_parts — reverted; attempted impact: The candidate removed a dead private parameter, but no change is retained because its benchmark evidence did not meet the performance gate.; evidence: Criterion dispatch_path comparisons reported repeated candidate regressions, from about 2.4% to 47.6% in affected cases…; reason: Independent workspace tests and strict lint/format passed, but five baseline/candidate dispatch benchmark runs repeatedly reported regressions beyond noise. The two files were restored byte-identically from backup.
…rs 500-recovery dedup, JNI streaming-setup-failure helper, two wire-header byte-identity tests (5 kept) Recovered from an interrupted iteration-4 IMPROVE batch: the driver was killed during item 8's implementation, leaving items 1-7's reviewed work uncommitted. Only the items that passed independent review are kept here. kept - item 1 (collector.rs): extract private `build_route_path` so the ROUTE_STORAGE fast path and the syn fallback cannot drift; replaces a `clippy::option_if_let_else` suppression with `map_or_else`. - item 2 (wire.rs): `build_header_vec` returns `Result<Vec<u8>, Vec<u8>>`, collapsing the duplicated 500-recovery blocks into one path. - item 3 (jni_impl.rs): private `streaming_setup_failed_array` replaces the duplicated error arms in dispatchStreaming / dispatchFullStreaming; JNI symbol names, signatures and error bytes unchanged. - item 5 (wire/tests.rs): hand-vs-serde byte identity over the heap fallback arms (40 header names, above STACK_CAP, all-single and mixed maps). - item 6 (wire/tests.rs): metadata fixture with quote / backslash / newline, compared on both the Vec and slice writer paths. reverted by review (restored from backup, not in this commit) - item 4 (jni_impl_support.rs): sequential `global_ref_checked` changed the null-check vs promotion-failure error precedence. - item 7 (streaming_closures.rs): evaluating `method_cache(env)` before the `can_call_unchecked` receiver check initialised the process-global cache on a null receiver. rolled back (never reviewed — implementation aborted mid-flight) - item 8 (RequestShape.java): restored byte-identically from .retry-now/backups/0004/item-08-8. verification: cargo fmt --check clean; cargo clippy --all-targets --all-features -- -D warnings exit 0; cargo test --workspace 0 failed.
…ared by header_read.rs decode/validate twins (BENCHMAR…, Route max_request_bytes through the shared read_env_clamped config helper, Ex… (4/6 applied) Applied (4/6): - [2] Unify the unicode-escape surrogate grammar shared by header_read.rs decode/validate twins (BENCHMAR… — impact: One private helper now owns surrogate parsing for decoding and validation, eliminating a documented accept/reject drift hazard without changing allocations or public behavior.; evidence: Five targeted runs were host-noisy but showed no repeatable affected-path regression; unknown-parser hand/serde ratio m…; decision: The reviewed diff preserves every parser branch and decoder-only scalar conversion. Workspace tests, clippy with denied warnings, formatting, and repeated affected benchmarks passed; see the review report. - [3] Route max_request_bytes through the shared read_env_clamped config helper — impact: The request-size ingress cap now uses the shared environment parsing policy, preventing future drift from the streaming settings while preserving its unlimited default and exact valid values.; evidence: No steady-state change expected: only the once-per-process OnceLock initializer changed. Two full benchmark runs comple…; decision: Independent diff review confirmed equivalent lookup, trim, parse, fallback, and identity-clamp behavior. Workspace tests, clippy, and formatting passed; benchmark runs found no candidate-attributable regression. - [5] Extract dispatch_streaming_with_header_body so both WithHeader JNI symbols share one shell — impact: The response-only header-streaming JNI symbol now delegates to a private body function through a matching argument carrier, aligning both WithHeader entry-point shells and reducing future control-flow drift.; evidence: none; the configured benchmark packages do not link vespera_jni, so no relevant benchmark median exists; decision: Independent backup diff review found a mechanical extraction only: callback order, runtime failure handling, buffer leases, Arc clone count, catch_unwind boundary, ABI, and panic fallback remain unchanged. Workspace tests, clippy, and form… - [6] Refresh the stale AGENTS.md KEY COMPONENTS table (11 missing modules, 5 drifted line counts) — impact: AGENTS.md now provides repository-relative navigation paths, current measured counts, and full coverage of the documented JNI and bridge implementation surfaces.; evidence: none; candidate changes Markdown only; decision: Independent checks found 37 component rows with zero missing paths, 35 numeric counts matching disk, and all 20 required files above 100 lines represented. Cargo tests and lint pass; see the review report for the unavailable oxlint and non… Not applied (2/6): - [1] Dedupe the WireHeaderStringSupport canonical-key tables — reverted; attempted impact: No change retained. The candidate's extra encoded table and comparison path expanded the hot-path implementation instead of delivering the requested minimal deduplication.; evidence: Rejected: reported heap byte[] lookup regressed 28-38% (about 1.4 ns); Rust benchmark run timed out during pass 2 of 5 …; reason: Independent review found the candidate deviates from the required String[][] plus existing-regionEquals design and its documented heap lookup regression is beyond noise. Java tests, allocation test, Rust tests, and lint/format passed; the … - [4] Collapse the two-hop normalize_path_key re-export chain in vespera_macro — reverted; attempted impact: The candidate removed two forwarding imports and pointed all users at file_utils, but it was rolled back because independent benchmark verification was incomplete.; evidence: none; benchmark verification incomplete; reason: Workspace tests and clippy/fmt passed, but five configured cargo bench runs exceeded the execution limit. The four target files were restored from the item backup; see the review report.
…n no response header name repeats, Extract the dispatchDirect0 jint result encoding into two shared helpers, Extract the duplicated di… (4/8 applied) Applied (4/8): - [1] collect_header_map: single-pass fast path when no response header name repeats — impact: Avoids a HeaderName hash and HeaderMap probe per distinct response header when names are unique, while preserving the repeated-header path.; evidence: No comparable end-to-end median: the configured benchmark lacks this envelope path and timed out amid unrelated host no…; decision: Reviewed the present single-file diff. The fast-path predicate exactly identifies no repeated names, preserves the existing set-cookie path, and passed workspace tests plus strict clippy and formatting. See the review report for benchmark … - [2] Extract the dispatchDirect0 jint result encoding into two shared helpers — impact: The direct JNI ABI encoders now have one private implementation each, preventing the two call-site pairs from silently drifting while preserving their sentinel fallback behavior.; evidence: none; the configured benchmark targets cannot execute the changed vespera_jni helpers; decision: Backup comparison confirmed a pure four-site extraction plus boundary tests. Independent workspace tests, strict Clippy, and format checks passed; the configured benchmark cannot reach vespera_jni helpers and exceeded the harness limit in … - [3] Extract the duplicated dispatch*WithHeader header callback into one factory — impact: Both JNI streaming-with-header symbols now share one private callback factory, preventing their header delivery and flag bookkeeping from drifting while preserving the exactly-once header contract.; evidence: none; configured benchmarks do not compile or link vespera_jni, so they cannot measure this JNI-only refactor.; decision: Independent code review found identical JNI call, result-to-flag update, and ownership transfer semantics. Workspace tests, clippy, fmt, and 13 targeted vespera_jni tests passed; details are in the review report. - [4] Collapse the seven repeated byte_array_from_slice conversions in jni_impl.rs — impact: A private JNI helper now centralizes seven identical wire byte-array conversions, preventing drift while preserving each caller's Result propagation and the distinct outer OOM fallback.; evidence: none; the configured vespera and vespera_inprocess benchmarks do not execute vespera_jni code; decision: Independent backup diff review confirmed only the requested extraction, unchanged wire error literals, and preserved exception clearing. cargo test --workspace and the clippy/format gate passed. The configured benchmark suite does not link… Not applied (4/8): - [5] Split collect_metadata_from_files and remove its clippy::too_many_lines allow — failed; attempted impact: Driver process died before this item was independently reviewed; retry-now recover rolled it back from its backup because it never passed the review gate.; reason: Driver process died before this item was independently reviewed; retry-now recover rolled it back from its backup because it never passed the review gate. - [6] Single HttpMethod name table backing both Display and TryFrom<&str> — skipped; attempted impact: Not attempted: the driver process died during item 5 of this batch.; reason: Not attempted: the driver process died during item 5 of this batch. - [7] Generalize parse_lit_str_slot into parse_slot<T: Parse> in args.rs — skipped; attempted impact: Not attempted: the driver process died during item 5 of this batch.; reason: Not attempted: the driver process died during item 5 of this batch. - [8] Deduplicate parse_request_body's extractor arms and drop its too_many_lines allow — skipped; attempted impact: Not attempted: the driver process died during item 5 of this batch.; reason: Not attempted: the driver process died during item 5 of this batch. Recovered by retry-now recover: the driver process died mid-batch, so this commit records the items that had already passed independent review. Item 5 was rolled back from its per-item backup because it never reached a review verdict.
… collector, Single-source the multipart WrongFieldType error construction, Dedup the four JNI setup_* promote/checkout routines, Singl… (5/5 applied) Applied (5/5): - [1] Dedup the per-file take-or-clone block in the collector — impact: One private helper now enforces identical move-or-clone handling for module and file paths in both collector paths, eliminating duplicated drift-prone logic without changing output.; evidence: none; the change is compile-time proc-macro code and does not enter the timed runtime benchmark paths; decision: Independent review found the helper semantically identical to both removed branches. Workspace tests, clippy with warnings denied, format, diagnostics, example rebuild, and byte-identical OpenAPI comparison all passed; see the review repor… - [2] Single-source the multipart WrongFieldType error construction — impact: A private helper now builds all scalar WrongFieldType errors, eliminating seven duplicate literals while preserving field names, expected types, and messages.; evidence: None measurable: configured benchmarks do not execute multipart scalar parsing.; decision: Independent diff review found equivalent construction at every call site. Workspace tests, Clippy with warnings denied, formatting, and the configured benchmark command passed; no benchmark reaches this parser. - [3] Dedup the four JNI setup_* promote/checkout routines — impact: Header streaming setup now owns only its header-specific validation and global reference, while shared reference, JVM, and checkout ordering lives in the non-header helpers. This makes the lease-safety invariant authoritative per stream sh…; evidence: none; the configured vespera/vespera_inprocess benchmarks do not compile or link the edited optional vespera_jni crate.; decision: Independent review confirmed both header helpers retain the original NullPtr identity and precedence, and checkout remains last fallible. Workspace tests, both mandated clippy modes, formatting, and the release JNI demo build passed. Full … - [4] Single-source the HTTP-method table across vespera_core and vespera_macro — impact: HttpMethod::ALL and as_str now provide one compiler-enforced source for every supported method name. Core parsing, formatting, and macro validation share it while invalid macro identifiers remain allocation-free.; evidence: none; no runtime benchmark path covers the changed method parsing or macro validation code; decision: Independent workspace tests, strict clippy/format, diff validation, and axum-example regeneration passed. The complete benchmark suite could not finish within the environment timeout; its runtime paths do not exercise this compile-time par… - [5] Remove the test-only struct-candidate index subsystem from the macro file cache — impact: Removed an isolated cfg(test) struct-candidate cache and its self-referential tests, reducing macro-cache maintenance surface while retaining coverage of the live file-list cache path.; evidence: none; decision: Independent diff review found only the two authorized files and no remaining removed-symbol references. Workspace tests, clippy, fmt, and the byte-identical axum-example OpenAPI build passed. Four benchmark runs completed; host variance wa… Not applied (0/5):
…pps/front, apps/admin do not exist) and two dead WHERE…, Correct crates/vespera_macro/AGENTS.md: drop the false anyhow convention, fix… (3/8 applied) Applied (3/8): - [1] Correct root AGENTS.md: frontend workspace (apps/front, apps/admin do not exist) and two dead WHERE… — impact: Repository guidance now points to real schema directories and the sole frontend workspace, so contributors use valid paths and commands.; evidence: none; decision: Independent path, package script, and stale-reference checks passed. The only candidate diff is AGENTS.md; workspace tests, Clippy with warnings denied, and formatting all passed. - [2] Correct crates/vespera_macro/AGENTS.md: drop the false anyhow convention, fix KEY FUNCTIONS locatio… — impact: Macro contributor guidance now names the actual syn::Error flow, production route scanner, current macro entry points, and complete module layout.; evidence: none - documentation-only change; no compiled or benchmarked artifact changes; decision: Independent inspection confirmed every requested location and all 20 module declarations. fmt, Clippy with warnings denied, and the workspace test suite passed. - [3] Correct crates/vespera_macro/src/parser/AGENTS.md for the schema/ and parameters/ module splits — impact: Parser documentation now reflects the split schema/ and parameters/ modules, eliminating dead paths, stale line counts, and obsolete entry-point names.; evidence: none; Markdown-only change with no compiled runtime impact; decision: Independent diff and path audit passed: all 42 documented paths exist, no line-count pattern remains, documented parser entry points resolve, and all configured tests, lint, format, and five benchmark runs exited successfully. See the revi… Not applied (5/8): - [4] Remove the dead err_spanned / IntoSynError doc references from crates/vespera_macro/src/error.rs — failed; attempted impact: Driver process died before this item was independently reviewed; retry-now recover rolled it back from its backup because it never passed the review gate.; reason: Driver process died before this item was independently reviewed; retry-now recover rolled it back from its backup because it never passed the review gate. - [5] method.rs tests: replace contains() token probes with an exact assertion and drive the exhaustive c… — skipped; attempted impact: Not attempted: the driver process died during item 4 of this batch.; reason: Not attempted: the driver process died during item 4 of this batch. - [6] Add try_set_operation return-value coverage in vespera_core (the predicate duplicate-route detectio… — skipped; attempted impact: Not attempted: the driver process died during item 4 of this batch.; reason: Not attempted: the driver process died during item 4 of this batch. - [7] Deduplicate the SCHEMA_STORAGE lock/entry/Arc::make_mut preamble shared by register_schema and inse… — skipped; attempted impact: Not attempted: the driver process died during item 4 of this batch.; reason: Not attempted: the driver process died during item 4 of this batch. - [8] BENCHMARK-AFFECTING (only one in this batch): short-circuit the default-app fast path in resolve_ap… — skipped; attempted impact: Not attempted: the driver process died during item 4 of this batch.; reason: Not attempted: the driver process died during item 4 of this batch. Recovered by retry-now recover: the driver process died mid-batch, so this commit records the items that had already passed independent review. Item 4 was rolled back from its per-item backup because it never reached a review verdict.
…relude into cap_and_split, route/utils.rs: collapse four duplicated literal-value blocks into a lit_value helper, wire/hoist.rs: extra… (8/8 applied) Applied (8/8): - [1] Extract the shared ingress-cap + wire-split prelude into cap_and_split — impact: One inline private helper now keeps the shared 413-before-400 prelude consistent across all owned-wire entry points while preserving each caller's error-delivery contract.; evidence: No credible regression. Five-run affected-path medians varied with host noise; immediate post-backup candidate confirma…; decision: Independent scope review confirmed the requested three call sites, preserved bidirectional exemption, and no API or wire changes. Workspace tests, all-feature clippy, no-default-feature clippy, and formatting passed; affected benchmarks sh… - [2] route/utils.rs: collapse four duplicated literal-value blocks into a lit_value helper — impact: Replaces four duplicate literal conversions with one private helper and confines the two necessary clippy suppressions to its five-line implementation.; evidence: none; decision: Independent diff review verified identical call inputs and outputs. Workspace tests, strict clippy, formatting, snapshot checks, and five runtime benchmark runs completed successfully; the runtime benchmark does not exercise this private p… - [3] wire/hoist.rs: extract the shared serde_json Value walk into hoist_from_value — impact: One private fallback helper now serves production and the benchmark-only DOM arm, removing duplicated extraction logic while preserving 422 selection and ordering semantics.; evidence: Five targeted samples were noisy, but all runs retained the expected typed-versus-DOM arm separation. No candidate-spec…; decision: Independent diff review confirmed the typed production fast path and full-DOM value_old arm remain separate. Workspace tests, clippy, fmt, and five targeted A/B benchmark runs passed; details are in the review report. - [4] file_cache.rs: stop holding the FILE_CACHE borrow across syn::parse_str — impact: parse_struct_cached releases its FILE_CACHE RefCell borrow before syn::parse_str, removing a latent re-entrant BorrowMutError panic risk in the proc macro while preserving behavior.; evidence: none; this proc-macro-only compile-time refactor is outside the runtime benchmark call graph; decision: Independent code review confirmed one unconditional counter increment before parsing, the unchanged signature and Result, and no live RefCell borrow across parse. Macro tests, workspace tests, strict clippy/fmt, and five configured benchma… - [5] internal.rs: convert two clippy too_many_arguments allow attributes to expect — impact: The two targeted suppressions now fail lint when they become stale, preserving their hot-path justification without changing runtime behavior.; evidence: none; lint attributes emit no runtime code; decision: Independent backup/diff review confirmed only the two requested attributes changed. Workspace tests, all-features lint, no-default-features lint, and formatting passed. The configured five-run benchmark exceeded the command limit without f… - [6] wire/tests.rs: add mixed object/array deep-nesting coverage for the ContainerStack overflow tier — impact: Adds parser-parity coverage for alternating object and array containers beyond the 128-level inline stack, including mismatched closer rejection in the heap overflow tier.; evidence: none; the only candidate change is inside #[cfg(test)] and cannot affect release benchmark artifacts.; decision: Independent diff review confirmed depth 200 crosses the overflow boundary and covers both container kinds. The targeted test, full workspace tests, clippy, and formatting all passed; details and the benchmark timeout limitation are in the … - [7] streaming_closures.rs: unify call_consumer_accept and call_future_complete — impact: One shared callback body prevents fast-path and exception-contract drift while preserving each wrapper's JNI method selection, return type, and fallback signature.; evidence: none; configured benchmarks do not compile or exercise the private vespera_jni helpers; decision: Independent code review verified all required behavior. Workspace tests, JNI all-feature build, clippy with warnings denied, and format check passed; five configured benchmark passes do not exercise vespera_jni. - [8] collector.rs: split the per-file body into fast/slow-path helpers and drop the too_many_lines allow — impact: Route discovery now isolates its stored-route and parsed-route paths in private helpers, leaving the outer collector focused on per-file setup. The blanket too_many_lines suppression is removed while preserving route metadata construction …; evidence: none; the change is confined to proc-macro compile-time code and does not affect runtime benchmark paths; decision: Independent diff review confirmed both extracted loops preserve field order, route order, description resolution, and take_or_clone semantics. Workspace tests and doctests, warnings-denied clippy, formatting, regenerated OpenAPI comparison… Not applied (0/8):
…nding-exception check-and-clear, vespera_macro: extract VesperaCache::is_fresh to dedup the cache-freshness predicate, vespera_macro: … (7/8 applied) Applied (7/8): - [2] vespera_jni: unify the three copies of the pending-exception check-and-clear — impact: One JNI-symbol-layer implementation now owns pending-exception checking and clearing, preventing future policy drift while retaining existing call-site behavior.; evidence: none; the configured vespera and vespera_inprocess benchmarks do not link the JNI-only changed crate.; decision: Independent diff review found the requested shared helper and wrapper. Exactly one check/clear pair remains in the three target files; fmt, both clippy configurations, the full workspace tests, and five configured benchmark invocations pas… - [3] vespera_macro: extract VesperaCache::is_fresh to dedup the cache-freshness predicate — impact: A single borrowed CacheKey and is_fresh predicate now govern both macro cache paths, preventing one path from silently omitting a future invalidation input while preserving export_app sidecar validation.; evidence: none; only compile-time proc-macro cache code changed, outside the benchmarked runtime dispatch paths; decision: Independent review confirmed the identical six comparisons and export-only sidecar check. Workspace tests, strict Clippy, formatting, cache hit/miss behavior, and the configured benchmark command completed; full evidence is in the review r… - [4] vespera_macro: extract finalize_metadata to dedup 4 copies of extend/merge/check — impact: One private helper now keeps metadata finalization identical across both vespera! and export_app! cache paths, removing a four-way drift hazard while retaining exact duplicate-schema diagnostics.; evidence: none (compile-time proc-macro refactor; runtime paths are unchanged); decision: Independent backup diff review found only the intended refactor and focused tests. Workspace tests, clippy, formatting, and axum-example build passed; generated OpenAPI and Insta snapshots were unchanged. - [5] vespera_macro: unify file-fingerprint computation on mtime+len — impact: Uses one mtime-plus-size fingerprint implementation for route files, sidecars, and macro sources. Timestamp-preserved source edits that change size now invalidate stale macro caches without extra metadata syscalls.; evidence: none; metadata syscall count is unchanged and no runtime dispatch path changed; decision: Independent review found the assigned diff correct. The size-only regression test, full workspace tests, clippy, formatting, and two axum-example builds passed. Five configured benchmark runs completed; their noisy runtime variance is unre… - [6] vespera_macro: extract with_current_crate_bucket in schema_impl.rs — impact: Centralizes poison-tolerant, per-crate copy-on-write schema-bucket mutation so the two mutating paths cannot drift while preserving their behavior.; evidence: none; this private proc-macro helper is outside the configured runtime benchmark binaries and retains the prior operati…; decision: Independent diff review confirmed unchanged duplicate and overwrite semantics; workspace tests, clippy, fmt, and five benchmark invocations completed successfully. See review report for evidence. - [7] vespera_macro: drop the normalize_path_key pass-through re-export chain — impact: Removes two misleading crate-internal re-export hops so every consumer imports normalize_path_key from its defining module.; evidence: none; compile-time import-only refactor with no runtime code or generated-token changes; decision: Independent diff review confirmed equivalent symbol resolution. Workspace tests, clippy -D warnings, fmt, and five benchmark runs all passed; full evidence is in the review report. - [8] vespera_jni: extract finish_header_dispatch to dedup the two dispatch*WithHeader tails — impact: Centralizes the identical post-dispatch JNI header tail and its load-bearing panic-path lease-discard invariant, reducing drift risk without changing behavior or public interfaces.; evidence: none; only private vespera_jni code changed, which is outside the configured vespera and vespera_inprocess benchmark bi…; decision: Independent review confirmed the Ok path releases the same leases in the same order and the Err path never invokes the release closure. cargo test, clippy -D warnings, fmt --check, and five configured benchmark runs passed. Not applied (1/8): - [1] wire header_write: single-distinct-name fast path in write_headers (BENCHMARK-AFFECTING) — reverted; attempted impact: No measurable target-path improvement was demonstrated; restoring the prior path avoids an extra branch on multi-header responses.; evidence: metadata_static_fast: 23.978 ns to 24.084 ns (+0.4%), within the reported 3-5% noise band.; reason: Workspace tests, clippy, formatting, and wire-contract goldens passed. The implementer’s interleaved target benchmark was +0.4%, within noise, and its analysis indicates LLVM already removes the work the fast path intended to avoid. See th…
…_include))] module attributes in vespera_jni, Delete the isHopByHopRequestHeader delegating wrapper and inline its three call sites, C… (6/6 applied) Applied (6/6): - [1] Remove the five redundant #[cfg(not(tarpaulin_include))] module attributes in vespera_jni — impact: Removes five redundant module cfg attributes already subsumed by the crate-level coverage gate, reducing maintenance noise without changing compiled behavior.; evidence: none; decision: Independent diff review found exactly five intended deletions. Workspace tests, all-feature clippy, fmt, tarpaulin-cfg check, and five benchmark invocations completed successfully; the change cannot affect normal-build codegen. - [2] Delete the isHopByHopRequestHeader delegating wrapper and inline its three call sites — impact: Removes a misleading private no-op wrapper while preserving request header filtering and documenting why the shared hop-by-hop predicate is correct.; evidence: none; the changed Java code is outside the configured Rust benchmark harness; decision: Independent diff review confirmed a pure static-delegate inline at all three required call sites. Java tests, Rust workspace tests, clippy, formatting, and Rust benchmark invocations passed; see the review report. - [3] Collapse HeaderPolicy's two duplicated ASCII-lowercase routines into one range-based fold — impact: One range-based ASCII fold now serves Connection-token parsing and header-name fallback, removing duplicate private logic while retaining the existing allocation profile and fast path.; evidence: none; Java-only refactor with unchanged one char[] plus one String allocation per folded value; decision: Backup comparison confirmed only the requested helper merge; Gradle tests, workspace tests, Clippy, formatting, and five benchmark runs completed successfully. See the review report for details. - [4] Replace the collector's double HashMap lookup with the entry API — impact: The collector slow path now hashes and probes each parsed file path once when storing and borrowing its AST, rather than repeating the lookup. It also eliminates the collector's Index panic site.; evidence: One fewer file-path hash and HashMap probe per collector slow-path file; runtime benchmark suite has no path through th…; decision: Independent diff review confirmed the owned key clone and AST borrow preserve the push_parsed_routes lifetime and behavior. cargo test --workspace, strict clippy/fmt, and five benchmark invocations passed; runtime benchmark noise is unrela… - [5] [BENCHMARK] Drop the redundant post-comma skip_ws() in the hand-rolled wire-header parser — impact: Removed two redundant whitespace scans after commas; read_string() performs the required skip at the next loop iteration.; evidence: Five-run median deltas: parse +0.45%; headers/1 -0.89%, /8 -2.24%, /16 -1.84%. No relevant regression exceeded +2%.; decision: Independent code review proved equivalence; workspace tests, clippy, fmt, and the five-run A/B benchmark passed. Details are in the review report. - [6] Table-drive WireHeaderStringSupport.canonicalKey, deleting six copy-pasted methods — impact: One length-indexed table replaces six duplicated canonical-key methods, so adding a supported header is a single table edit while preserving shared literal instances and concrete hot-path comparisons.; evidence: Java p3_apply allocation is 576 bytes/op. Rust Criterion benches are structurally unrelated because no Rust code change…; decision: Independent diff review confirmed the table exactly matches every prior length and literal. Forced Gradle tests, the Java allocation benchmark, cargo test, cargo fmt, and cargo clippy all passed; full evidence is in the review report. Not applied (0/6):
…erMap into a LazyLock static (PERF item), parse_request_body: collapse the four copy-pasted extractor arms, remove the unwrap() panic … (6/8 applied) Applied (6/8): - [1] error_wire: hoist the fixed content-type HeaderMap into a LazyLock static (PERF item) — impact: Error wire construction reuses an immutable one-entry content-type HeaderMap, eliminating a per-response allocation and header-name hash without changing the emitted bytes.; evidence: No directly exercised Criterion group covers error_wire. Five full benchmark runs passed; successful-path variation is …; decision: Independent source review confirmed the prescribed scoped change and unchanged write path. Focused and workspace tests, clippy, and formatting passed; wire_contract locks error_wire bytes exactly. Five benchmark runs completed, with unrela… - [2] parse_request_body: collapse the four copy-pasted extractor arms, remove the unwrap() panic site, d… — impact: Consolidates duplicated request-body construction, removes the macro-path panic site, and eliminates an unnecessary clippy suppression while preserving deterministic OpenAPI output.; evidence: none; this proc-macro-only refactor is outside the runtime dispatch benchmark's execution path. The attempted five-pass…; decision: Independent diff review found behaviorally equivalent private helpers. Targeted 19-test parser suite, full workspace tests, strict clippy, and fmt check pass; no pending Insta snapshots or diff whitespace errors. - [3] rename_field: extract one helper per rename_all strategy and drop the too_many_lines allow — impact: The long rename_field implementation is now a concise strategy dispatch over six private helpers, and the clippy suppression is removed without merging intentionally different case algorithms.; evidence: No candidate runtime path: this private proc-macro refactor does not reach vespera or vespera_inprocess benchmarks.; decision: Independent backup diff review found only a behavior-preserving extraction. Scoped and workspace tests, strict clippy/format checks, and the configured benchmark command all completed successfully; detailed evidence is in the review report. - [4] impl Serialize for Schema: split the 170-line field emitter into ordered group helpers and drop the… — impact: Schema serialization is split into six private ordered field-group helpers, removing the too_many_lines suppression while preserving the OpenAPI field sequence.; evidence: none; configured benchmarks do not construct or serialize Schema; decision: Independent review confirmed the exact helper grouping and order. Workspace tests, Insta snapshots, clippy, formatting, example OpenAPI regeneration, and the configured benchmark completed successfully; see the review report. - [5] Remove the path.segments.last().unwrap() panic sites in vespera_macro parser modules — impact: All six empty-path parser cases now return their existing neutral fallback, and the two String type defaults no longer parse with unwrap. This prevents opaque proc-macro panics without changing valid-input output.; evidence: none; decision: Independent diff review confirmed scope and fallback equivalence. cargo test --workspace, clippy -D warnings, and fmt --check passed; the executable non-test panic scan is clean. The repeated benchmark sweep timed out during its final repe… - [6] normalize_display_path: drop the unconditional second allocation — impact: Avoids the unconditional replacement allocation for separator-free paths while preserving normalized display output.; evidence: One allocation removed for valid UTF-8 paths without backslashes; configured runtime benches do not link this proc-macr…; decision: Independent review confirmed equivalent lossy path rendering; targeted and workspace tests, clippy, formatting, and five benchmark-suite runs passed. See the review report. Not applied (2/8): - [7] Replace the named-field ident.unwrap()/expect() panic sites with spanned diagnostics — failed; attempted impact: Machine failure: no valid signal; reason: Driver terminated item 7: no valid signal - [8] Remove the 'infallible' expect()/unwrap() sites in schema_macro/codegen.rs, schema_macro/type_utils… — skipped; attempted impact: not attempted because item 7 terminated the batch; reason: not attempted because item 7 terminated the batch
demo-app pinned kr.devfive:vespera-bridge 0.1.1 while the in-repo bridge moved to 0.2.0, so every JNI E2E job compiled the E2E tests against the last Maven Central release and died on 27 "cannot find symbol" errors (dispatchDirect, dispatchDirectPooled, BufferTooSmallException, DecodedResponse.bodyBytes / ByteBuffer body). Both the bridge dependency and the Gradle plugin now resolve the version declared in their own build scripts, so a bump can no longer fall back to a published artifact - the plugin is not even on the Plugin Portal, so that fallback is fatal. With compilation restored, two behaviour changes that landed while the gate was blind surfaced as failures: - dispatchAsync retried CompletableFuture.complete() whenever the first attempt reported an error. jni's call_method_unchecked POST-checks the pending exception, so a throwing complete() is reported as Err only after the Java method has already run: the retry re-entered the same broken method and completed the same future twice. complete_future now returns CompleteAttempt and the retry runs only when the response byte[] could not be allocated, i.e. complete() was never reached. - StreamingClosureStressTest still expected the handler's 200 from a dispatch whose OutputStream throws. dispatch_response_streaming has surfaced a sink stop as 500 since "Check java buffer" - a truncated body must never be reported as a clean success - and the Rust side locks that in response_streaming_chunk_break_returns_500_not_silent_success. The Java expectation is updated to the same contract. Also corrects the pending-exception docs in streaming_closures.rs, which claimed the cached call_method_unchecked path returns Ok with the throw left pending. That mis-statement is what made the double-complete look correct.
Coverage is measured the way CI measures it: `cargo fmt` with the wide `.rustfmt.toml` CI writes, then `cargo tarpaulin --engine llvm`. Under that harness the workspace went from 6458/6917 (93.36%) to 6837/6890 (99.23%). Two structural changes: - tarpaulin.toml excludes `*benches*`. The criterion drivers and the macro-compile-bench fixture crate are instrumented but never run, so they only ever contributed 0%-covered lines; the number now describes shipped code. - The tests themselves: ~2.7k lines of new assertions across every crate, targeting the error/fallback/edge arms that had no exercise - serde visitors and schema (de)serialization in vespera_core, the Validated and TypedMultipart rejection paths in vespera, config clamping / wire header parse+write / 422 hoisting / streaming failure arms in vespera_inprocess, and the proc-macro parser, route, schema_type! and vespera! internals in vespera_macro. Every added test asserts real behaviour (a parsed value, a serialized shape, a `syn::Error` message, an HTTP response); no production code path was changed to make a test pass, and no snapshot or wire-contract expectation was relaxed. Two pre-existing defects surfaced while writing the tests and are fixed here: `schema_impl/tests.rs` filtered `compile_error!` by bare ident, but `syn::Error::to_compile_error` emits the fully-qualified `::core::compile_error!`, so the assertion silently compared against an empty list; and `parser/response/tests.rs` took a reference where clippy's `borrow_as_ptr` wants `&raw const`.
Follow-up to the 99.2% pass. Adds the last targeted tests (route-arg defaults, serde-attr token-string fallbacks, schema_macro circular/file_cache/type_utils edges, vespera_impl cache + openapi_io arms, oversized header maps) and removes four branches that could never execute: - SchemaTypeWire::into_schema_type_and_nullable returned Option<bool> but could only ever produce None or Some(true). It now returns bool, which deletes the dead Some(false) arm from BOTH consumers instead of leaving them written against a state that does not exist. - apply_schema_field's SchemaField::RefPath arm was dead because visit_map handled \ itself. visit_map now guards that arm (if pure_ref && ref_path.is_none() && !has_inline_fields) so a repeated \ falls through to the shared arm - same observable behaviour, one less duplicated assignment, and the previously dead arm is now the one that handles duplicates. - Schema::from_compiled_json asserted debug_assert!(false, ..) before returning its sentinel. The surrounding doc promised it 'never panics in downstream user code', but the assert fires in exactly that case: this constructor runs inside macro-generated code in the USER's crate. The assert is gone; the loud VESPERA_SCHEMA_PARSE_ERROR sentinel remains and is now actually tested. - header_write dropped two unreachable debug_asserts on the response hot path of the FFI bridge, where an assert would take the host process down over a missing header or a pointer-equality invariant that cannot fail. The duplicated f64 -> i64 -> f64 round-trip check behind integer-form number constraints is now one lossless_integer_form helper.
- file_utils::normalize_path_key: the `\\\\?\\` verbatim-prefix strip and the case fold are now behind #[cfg(windows)] instead of a runtime cfg!() branch. They are Windows-only concepts, so Unix builds stop paying a prefix scan and stop carrying two lines no Linux run can execute. Adds the missing dot-segment folding test. - is_keyword_type: covers the zero-segment path guard with a hand-built TypePath (syn cannot parse an empty path). - wire header parse: adds the missing duplicate `app` and duplicate `v` rejection cases alongside the existing duplicate-key table. - hoist: covers try_hoist_validation_errors directly for the non-JSON content-type and over-cap body skips, not just the bench twin. - seaorm::convert_type_with_chrono: re-emits the wrapper ident directly instead of matching it back with an unreachable!() arm. option_or_vec_inner only ever reports Option or Vec, and the crate forbids panics on macro paths - a proc-macro panic has no span and surfaces as an opaque 'proc macro panicked'.
Path::components() normalizes inner '.' away on its own and only yields Component::CurDir for a LEADING '.', so the folding loop's CurDir arm is reachable only while the joined path stays relative. The previous case joined against an absolute cwd and therefore never reached it.
Both were flagged by the coverage pass and then verified by hand rather than trusted:
- generate_type.rs: the HasMany inline-generation failure arm re-tested pick_set.contains(..), but the guard ~25 lines above already returns early for any field NOT in pick_set. The condition was therefore always true and the trailing `continue` unreachable, so the error is now unconditional.
- schema_assertions.rs: collect_leaf_custom_types returns early for both shapes that lack a final path segment, so last_path_ident could never return None for a leaf it yielded. The function now returns each leaf paired with that ident, which makes the invariant structural and removes the unreachable `else { continue; }` at the call site. last_path_ident had no other production caller and is gone; its test is replaced by one asserting the pairing.
tarpaulin's llvm engine emits a gap region for the join point that follows a diverging block (a `return` inside an `if`, a `let ... else`, a `match` arm that returns), and that region inherits the counter of the branch that diverged. When the early-return branch is unreachable, the closing brace - or the first statement after it - reads 0 hits even though it runs on every call. Two fixes apply, and both are used here: 1. Cover the diverging branch for real. An oversized borrowed direct-write request, a malformed borrowed wire frame, a 422 body that errors mid-collect, a wire header with "v":-1, a malformed number, and a container-stack pop at depth 0 now all have tests. 2. Extract the post-divergence code into a named function otherwise: envelope_serialization_fallback, the buffered body drain, the wire-header backfill / overflow / body-append tails, collect_sorted_header_entries, write_current_metadata, build_struct_metadata, find_bare_struct_in_file, collect_rs_entry, macro_src_dir, fingerprint_rs_directory, collect_rs_mtime_entry, hash_security_scheme. Several of those extractions are what makes previously untestable code testable at all: build_struct_metadata and find_bare_struct_in_file now take the call-site file as a parameter instead of reaching for Span::call_site().local_file(), which is None outside a real macro expansion, and macro_src_dir / fingerprint_rs_directory take the directory instead of resolving it from the build's target dir. Also lifts validated.rs's unreachable 422 fallback literal into a named FALLBACK_VALIDATION_ENVELOPE const with a test that finally asserts the field-order claim its comment had only ever stated, and promotes SchemaFieldVisitor out of the deserialize body to module scope. Wire bytes, allocation budget, insta snapshots and trybuild diagnostics are unchanged; collect_sorted_header_entries keeps the stack path allocation-free by taking the heap Vec as a parameter rather than returning an owned one.
Follow-up to the previous split: the ten lines still uncovered were all the
join point after a block that ends in `return` / `continue`, whose gap region
inherits the never-taken branch's counter. Rewriting each as a single
expression removes the join point entirely, and none of them changes
behaviour:
- dispatch: `if !write_wire_header_into_vec(..) { return .. }` becomes an
if/else expression; the `unwrap_or_else` fallback comment moves above the
statement it explains so the closure is a single forwarding line.
- wire: `let Ok(header_len) = .. else { return false }` becomes `is_ok_and`,
and `to_wire_bytes`'s trailing `match` becomes `map_or_else`.
- file_lookup: `let Some(file_path) = Span::call_site().local_file() else {..}`
becomes `map_or_else`, and both `.rs` collectors swap
`let Ok(file_type) = .. else { continue }` for `if let Ok(..)`.
- cache: `hash_security_scheme`'s two-arm `match` becomes
`.as_deref().unwrap_or("scheme:unserializable").hash(..)` (`Hash` for
`String` delegates to `str`, so the hash is unchanged), and
`compute_macro_dev_fingerprint_uncached`'s `let ... else { return 0 }`
becomes `map_or(0, ..)`.
Also moves `collect_rs_mtime_entry`'s metadata-sentinel note into its doc
comment rather than leaving it as the first line of a branch body.
The last uncovered line was `entry.metadata().as_ref().map_or(0, file_fingerprint)`: the `0` default is a scan-time race guard that a real `DirEntry` cannot provoke, so its region never executed. The same expression appeared verbatim in both compile-time scanners (`file_utils::collect_with_mtimes_into` and `vespera_impl::cache::collect_rs_mtime_entry`). It is now one `file_utils::entry_fingerprint` that takes the `io::Result<Metadata>` as a parameter, which both dedups the sentinel - it can no longer drift between the two scanners - and makes the failure arm reachable from a test.
The else-if chain in collect_rs_mtime_entry was the last line tarpaulin refused to credit, even though the CI run shows collect_rs_mtime_entry_uses_resolved_file_type passing - and that test can only assert one fingerprint if the branch body ran. Splitting the .rs filter into push_rs_fingerprint leaves a plain if/else whose arms are both directly exercised, and puts the extension filter next to the fingerprinting it guards.
vespera_impl::cache carried its own recursive `.rs` scanner (collect_rs_mtimes / collect_rs_mtime_entry / push_rs_fingerprint) that duplicated file_utils::collect_with_mtimes_into almost line for line: same DirEntry::file_type dispatch, same "skip the metadata() stat for non-.rs files", same mtime+size fingerprint. It now reuses file_utils::collect_files_with_mtimes and keeps only the part that was actually its own - hashing the `.rs` subset in sorted order, split out as hash_rs_fingerprints. That deletes three functions, removes the duplicated walk (the two copies could have drifted on the sentinel or the stat-skipping rule), and drops the last branch tarpaulin would not credit. `fingerprint_rs_directory` keeps its contract: a missing or unreadable directory hashes to 0. The tests move with it - they now drive fingerprint_rs_directory and hash_rs_fingerprints instead of the deleted helpers, and the nesting case is covered explicitly (a `.rs` file in a subdirectory must reach the digest).
The jni-e2e job published the bridge JAR but never ran its tests, so ~2 kLOC of Java - the wire codec, the zero-copy header reader, the direct buffer pool, the hop-by-hop header policy, the dispatch-mode resolvers and the Spring autoconfiguration - shipped with 175 tests that no pipeline executed. They now run on all three OSes before the publish step, and their XML lands in the same artifact as the demo-app results. Also corrects the job header, which still described a hard-coded kr.devfive:vespera-bridge:0.1.1 pin that no longer exists, and records why this job matters beyond Java: crates/vespera_jni is excluded from tarpaulin because its symbols need a live JVM, so this job - not the Rust line-coverage number - is what covers it.
The Java side had no coverage measurement at all. JaCoCo now runs on
libs/vespera-bridge and reports in CI: 67.7% -> 94.8% line, 59.6% -> 89.9%
branch.
Three things got it there:
1. ~250 new unit tests across the wire codec, the zero-copy header reader, the
direct buffer pool, the native loader, the hop-by-hop header policy, the
dispatch-mode resolvers, the configuration properties and the Spring
autoconfiguration.
2. Merged execution data. The JNI wrappers and the Spring proxy only run with a
real cdylib loaded, which the unit suite cannot do - but the demo-app E2E
suite does. demo-app now records a JaCoCo exec file and the bridge's report
merges it, so the number describes what actually ran rather than only what
the unit tests reach.
3. Spring integration tests. VesperaProxyController is the entry point this
library exists for - an autoconfigured catch-all that forwards every request
into Rust - and it had never been driven by an actual HTTP request. It now is,
through a real Tomcat and a real cdylib, for every dispatch mode, app
selection, unknown/invalid app, handler panic, HEAD, unknown route, and the
controller-disabled case.
Writing (3) exposed a real defect: the integration pattern the READMEs tell
users to copy does not start.
@SpringBootApplication
@componentscan(basePackages = {"com.example.app", "com.devfive.vespera.bridge"})
Component-scanning the bridge package makes Spring instantiate
VesperaProxyController as a plain @RestController. It declares four constructors
and no default one, so the context dies with
BeanCreationException: Error creating bean with name 'vesperaProxyController'
Failed to instantiate: No default constructor found
Caused by: NoSuchMethodException: VesperaProxyController.<init>()
The scan is also wrong even where it happens to work: the controller is
registered by autoconfiguration, so a scanned copy bypasses
vespera.bridge.controller-enabled and every @ConditionalOnMissingBean override.
DemoApplication and all three READMEs drop the scan;
DemoApplicationContextTest boots the real DemoApplication as the regression
guard (verified: it fails with the annotation restored, passes without).
Two of the new proxy tests also document HTTP-level constraints that are easy to
get wrong: a streamed response carries no Content-Length, and a full-duplex
echo through /echo/stream must stay under the socket buffers because a blocking
HTTP client will not read the response until it has finished writing the
request. Large-body coverage of that dispatch mode uses the buffered /echo
route instead.
94.8% -> 100.00% line (1830/1830), 89.9% -> 95.9% branch, 284 -> 316 tests. The residue was the same shape the Rust side hit: failure modes a real caller cannot provoke. Two levers closed them, and neither weakens a test. Reachable branches simply got tests: buffer growth and adaptive shrink, the 2 GiB and 64 MiB buffered-body limits, short EOF, known vs unknown Content-Length, HEAD and 204 body suppression, the compatibility constructors, ASYNC success / executor rejection -> 503 / wrapped failure / cyclic cause, and the DIRECT overflow -> remember -> downgrade-to-STREAMING sequence driven through real HTTP. Unprovokable ones got an injectable seam - the decision moved into a small package-private method that takes its input as a PARAMETER, so a unit test can supply the value the environment never will: - VesperaNativeLoader: messageDigest(algorithm), requireMatchingSize(..), requireMatchingDigest(..), deleteAfterFailedLoad(path, deleter). SHA-256 being absent, a copy whose size or digest does not match, and a failed temp delete after a failed load are all now testable. - VesperaBridge: the bundled-vs-system load decision, pending-vs-property configuration, and the optional native symbols (configureStreaming0 / configureRuntime0 are absent on pre-0.2 libraries). - VesperaDirectBufferPool: the reflective virtual-thread probe and the JNI return-code classification - success, first and second overflow, retry, oversized, and Integer.MIN_VALUE - now take the int rather than calling native code. - VesperaProxyController: effectiveMode(..), shouldRetryDirect(..) and the two buffered-body limit checks. One unreachable guard was deleted rather than papered over: a post-overflow negative-code check in the buffer pool that the branch above it had already handled. Also restores three comments the extraction dropped, which had left empty catch blocks behind. They document why the swallow is deliberate: pre-0.2 native libraries do not export the optional tuning symbols, and a failed temp-file delete must not mask the load failure the caller is about to report.
95.91% -> 100.00% branch (1111/1111), line stays 100.00% (1830/1830).
Most of the 46 were simply untested and got tests. The rest were not
reachable at all, and those split into three kinds.
A real bug. detectOs tested os.name for "win" before "darwin", but the
string "darwin" CONTAINS "win" -- so a Darwin-reporting JVM was told it
was Windows and went looking for a .dll. The darwin arm could never run.
Worse, the test added in the previous commit had frozen the defect as the
expected value:
assertDetection("Darwin", "aarch64", "windows", "aarch64", "demo.dll");
That is what asserting observed behaviour instead of intended behaviour
buys you. The check order is now mac/darwin first, and the assertion says
macos + libdemo.dylib.
Conditions that could not be false. Two `n != Integer.MIN_VALUE` guards
sat directly under an `n == Integer.MIN_VALUE` that had already thrown;
`cap < 0` guarded an int overflow that cannot happen, because the loop
only doubles while cap < needed <= 64 MiB; and the DIRECT overflow
handler re-tested `isSafe(method)`, which the early return at the top of
the method had already established. That last one now reuses the same
`retry` value the dispatch was issued with, hoisted out of the try, so
the decision exists once instead of twice.
Compiler-generated branches. javac copies the try-with-resources
resource-close sequence once per normal return, and readBody had four --
so the closing brace carried four `ifnull` checks on a resource
`request.getInputStream()` never returns null for. Extracting
readBufferedBody leaves the try with a single exit, which is the shape
JaCoCo's try-with-resources filter recognises. The suppression semantics
are unchanged: it is still try-with-resources, so a close failure during
an in-flight exception is still suppressed rather than replacing it.
New tests cover the 3-byte UTF-8 boundary in both directions (overlong
E0 80 80, surrogate ED A0 80, and the valid E0 A0 80 between them), a
high surrogate followed by a plain character rather than another escape,
negative numbers in both value and skipped position, and a punctuation
byte below '0' in both -- ':' only exercised the upper bound.
No test was disabled, skipped, or weakened, and no coverage exclusion was
added; the branch total falls 1125 -> 1111 purely because the impossible
conditions are gone.
tarpaulin cannot do this at all -- its own help says `-b, --branch Branch coverage: NOT IMPLEMENTED` -- so this adds a separate nightly cargo-llvm-cov job. It is deliberately NOT a gate, and the job comment says why at length, because the obvious next move on seeing a sub-100% number in CI is to set a threshold on it. The number under-counts. rust-lang/rust#124118 (open) lists as not yet instrumented: match arms and or-patterns, the `?` operator, `.await`, and any branch introduced by macro expansion -- "the current implementation discards any branch span that isn't directly visible in the function body". vespera_macro owns roughly four fifths of the measured branches and exists to emit code inside `quote!`, so those branches are not reported as uncovered; they never enter the denominator. Driving this to 100% would mean reshaping match/`?`/macro code until the instrumenter happens to see it, which is the opposite of what the line-coverage work did. Measured on this workspace at 81.26% branch (608/3244 missed). Note it is not comparable to the tarpaulin line number: llvm-cov counts 24,313 lines where tarpaulin counts 6,859, so the two line percentages disagree without either being wrong. `vespera_jni` is excluded for the same reason tarpaulin excludes it -- no JVM, no execution -- and jni-e2e is its real coverage. Left in, it reports 4% branch and buries every other crate. Two things the job has to tolerate. It pins a moving nightly, so the instrumented run is `continue-on-error`: the trybuild UI suite blesses .stderr against stable and a nightly diagnostic reword fails it with nothing broken. The profraw is still written, so the report step runs anyway. And `--branch` goes on the instrumented run only -- `report` rejects the flag, while still emitting branch columns because the profdata carries the counters. changepacks still needs only [test, jni-e2e], so this can never hold up a release.
Triaged all 409 uncovered-branch sites the observational job reported and
tested the ones that turned out to be real. No production code changed --
every addition sits inside a `#[cfg(test)]` module.
The most useful thing the signal found is in vespera_core's OpenAPI merge.
`merge_path_item` is a run of `if into.put.is_none() { ... }` per field, and
only the "slot was empty, fill it" side had ever executed. The untested side
is the actual contract of `merge = [...]`: a child app must not overwrite an
operation the parent already defined. Inverting any one of those conditions
would have silently clobbered parent operations with no test objecting.
Same shape for top-level `security` and `external_docs`.
Elsewhere the real gaps were mostly malformed input. enum_repr parses
`#[serde(tag = ...)]` by hand with `find('=')` and `find('"')`, and the
untested sides were all the ways that string can be malformed -- a proc
macro that mishandles those fails at compile time with a terrible message.
Similar for multipart `limit`/`field_name`, the router DSL's duplicate and
empty lists, and the compile-time filesystem walk (a directory that looks
like a module, a path that is not a file, a stale cache entry).
A large share of the remaining sites are not production code at all. This
repo keeps unit tests inline in `#[cfg(test)] mod tests`, and the branch
instrumenter counts those too -- a test's own `let Inline(s) = r else {
panic!() }` registers an uncovered branch for the arm that would have failed
the test. Most of type_schema.rs, enum_schema.rs and conversion.rs is that.
Those are noise in the denominator, not missing tests.
Two findings worth recording, neither fixed here.
serde applies rename_all differently by position: a struct field is assumed
snake_case already, an enum variant PascalCase. Measured against serde 1,
`SCREAMING_SNAKE_CASE` turns variant `USER` into "U_S_E_R" but field `USER`
into "USER", and `camelCase` turns variant `XMLParser` into "xMLParser"
while this crate produces "xmlParser". `rename_field` serves both positions
through one implementation, so it cannot match serde in both. Idiomatic
names agree, which is why nothing has hit it. The new cases pin the current
behaviour and say which rule they are pinning.
Six `needless_raw_string_hashes` came in with the new fixtures and would
have failed CI, which lints with -D warnings. Fixed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.